durable task scheduler auth extension support #362
Conversation
save initial
|
@jviau would you mind taking a look at this as well? The main feedback I'm interested in from you is the package name/organization. The idea behind this PR is to introduce a mechanism for configuring the gRPC channels to work with the Durable Task Scheduler (DTS). This includes setting up the appropriate auth and some other service-specific headers (like the task hub name). I figured this might be the right time to introduce an "Extensions" concept and make "Azure" the extension consumers should use for talking to DTS. There could be others in the future. WDYT? |
This reverts commit 131c575.
I wonder if we want a name other that Extensions for backends? I am worried this may get confused with a backend vs helpers for Azure / a specific Azure service. Say we wanted to port an AzureStorage backend hook to durable - where would that go? Or if we wanted to introduce helpers (not backends) for interacting with Azure services (Compute, Storage, CosmosDB, etc). Where would those go? There is the This also brings up the point of keeping a separate Worker and Client package. While it is more setup work on our end, the benefit is customers can bring in just the clients for their API projects and not need the entire worker. |
|
split into separate packages and split tests, also add @cgillum 's sample to here using split packages and tested working |
test/Client/AzureManaged.Tests/DurableTaskSchedulerClientExtensionsTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jacob Viau <[email protected]>
YunchuWang
force-pushed
the
wangbill/auth
branch
from
821acdd to
c697fbc
Compare
src/Client/AzureManaged/DurableTaskSchedulerClientExtensions.cs
Outdated
Show resolved
Hide resolved
test/Client/AzureManaged.Tests/DurableTaskSchedulerClientExtensionsTests.cs
Outdated
Show resolved
Hide resolved
test/Client/AzureManaged.Tests/DurableTaskSchedulerClientExtensionsTests.cs
Show resolved
Hide resolved
test/Client/AzureManaged.Tests/DurableTaskSchedulerClientExtensionsTests.cs
Outdated
Show resolved
Hide resolved
test/Client/AzureManaged.Tests/DurableTaskSchedulerClientExtensionsTests.cs
Outdated
Show resolved
Hide resolved
test/Client/AzureManaged.Tests/DurableTaskSchedulerClientOptionsTests.cs
Outdated
Show resolved
Hide resolved
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Azure.Identity" Version="1.13.1" /> | ||
| <PackageReference Include="Microsoft.Extensions.Options.DataAnnotations" Version="8.0.0" /> |
There was a problem hiding this comment.
We are targeting net6 but using a v8.x package. While this is technically 'valid', it may be confusing to users. Are we able to use a 6.x package here?
Side note: targeting net8 and using v8.x packages would be another discussion / work item. Don't want to get that tangled into this PR.
There was a problem hiding this comment.
v6 of this package does not target dotnet 6, and v7 does not have
validateonstart. v8 is min version targeting dotnet 6 and has validateonstart api
There was a problem hiding this comment.
Do you need ValidateOnStart? This will effectively be validated on start, as validation will occur on first import of it, which will be done at start due to this being eventually imported by a IHostedService (the DurableTaskWorker)
There was a problem hiding this comment.
good point, updated ver and removed validateonstart
test/Client/AzureManaged.Tests/DurableTaskSchedulerClientExtensionsTests.cs
Outdated
Show resolved
Hide resolved
test/Client/AzureManaged.Tests/DurableTaskSchedulerClientExtensionsTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
All my main concerns are resolved. Thanks @YunchuWang! I agree with Jacob's recent round of feedback too, so let's address those (I'll leave it up to you to decide on whether to take on some of the code de-duplication, I don't have a strong opinion on this) and then I think we're good to merge this.
This pull request introduces several significant changes, primarily focusing on the setup of new projects and the integration of Azure Managed extensions for the Durable Task Framework client. The most important changes include the addition of new projects to the solution, updates to the build configuration, and the implementation of new client extensions and options for Azure Managed services.
New Projects and Build Configuration:
Worker.AzureManaged,Client.AzureManaged,Shared.AzureManaged.Tests,Client.AzureManaged.Tests, andWorker.AzureManaged.Teststo the solution fileMicrosoft.DurableTask.sln.Microsoft.DurableTask.slnto include the newly added projects. [1] [2].github/workflows/validate-build.yml.Azure Managed Client Implementation:
Client.AzureManaged.csprojproject file with necessary package references and project dependencies.DurableTaskSchedulerClientExtensionsto provide extension methods for configuring Durable Task clients to use the Azure Durable Task Scheduler service.DurableTaskSchedulerClientOptionsto configure the Durable Task Scheduler, including methods for creating gRPC channels and handling connection strings.AccessTokenCacheto manage and refresh Azure access tokens.DurableTaskSchedulerConnectionStringto parse and handle connection strings for the Durable Task Scheduler service.